Skip to content

Fix multiple requests on same HTTP1 connection#78

Open
gjcairo wants to merge 9 commits intomainfrom
http1-fixes
Open

Fix multiple requests on same HTTP1 connection#78
gjcairo wants to merge 9 commits intomainfrom
http1-fixes

Conversation

@gjcairo
Copy link
Copy Markdown
Collaborator

@gjcairo gjcairo commented Apr 2, 2026 •
edited
Loading

We're not currently handling keep alive HTTP1 connections properly: we close connections at the end of the first request, yet we don't send a connection close header. This PR fixes that.

This also removes 6.2 support because there are some compiler issues with sendability that have not been properly addressed.

@gjcairo gjcairo added the 🆕 semver/minor Adds new public API. label Apr 2, 2026
aryan-25
aryan-25 reviewed Apr 2, 2026
View reviewed changes
Comment thread Sources/NIOHTTPServer/HTTPRequestConcludingAsyncReader.swift Outdated
@guoye-zhang
Copy link
Copy Markdown

guoye-zhang commented Apr 2, 2026

I pointed apple/swift-http-api-proposal#137 to the latest commit on this branch, but the AHC conformance test is still failing it appears

@gjcairo
Copy link
Copy Markdown
Collaborator Author

gjcairo commented Apr 7, 2026

@guoye-zhang hm, interesting. I'll take a look at that.

@0xTim
Copy link
Copy Markdown
Member

0xTim commented Apr 9, 2026

Just to but in here, this has fixed a load of test failures in Vapor that were failing with Caught error: I/O on closed channel so thank you!

@gjcairo gjcairo requested a review from aryan-25 April 13, 2026 16:44
@gjcairo
Copy link
Copy Markdown
Collaborator Author

gjcairo commented Apr 13, 2026

@guoye-zhang the failing conformance test now passes for me.

@guoye-zhang
Copy link
Copy Markdown

guoye-zhang commented Apr 14, 2026

I updated apple/swift-http-api-proposal#137 and it's still running into these errors with AHC conformance tests: ✘ Test testOk failed with error: I/O on closed channel

@gjcairo
Copy link
Copy Markdown
Collaborator Author

gjcairo commented Apr 14, 2026

testOk and testBasicRedirect are flaky, but they only fail like twice if I run them 100 times.
Are you seeing them fail more consistently than this? Are we sure this is an issue on the server side?

@guoye-zhang
Copy link
Copy Markdown

guoye-zhang commented Apr 14, 2026

I'm actually reproducing it 100% on my Mac running the test in Xcode. GitHub CI reproduced it as well. Previous workaround with Connection: close never failed for me.

gjcairo added 7 commits April 24, 2026 11:04
@gjcairo
Fix multiple requests on same HTTP1 connection
637487d 
# Conflicts:
#	Sources/NIOHTTPServer/NIOHTTPServer+HTTP1_1.swift
#	Sources/NIOHTTPServer/NIOHTTPServer+SecureUpgrade.swift
#	Sources/NIOHTTPServer/NIOHTTPServer.swift
@gjcairo
Format
4e5ad11
@gjcairo
Exit request loop if task is cancelled
cfc55a4
@gjcairo
Simplify iterator sharing cycle
fd80541
@gjcairo
Fix flaky test
3f0a85a
@gjcairo
Disable 6.2
0362155
@gjcairo
Drain request when not done reading
89ec767
@gjcairo gjcairo force-pushed the http1-fixes branch 2 times, most recently from b96e87c to 4947a0d Compare April 30, 2026 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aryan-25 aryan-25 Awaiting requested review from aryan-25

1 more reviewer

@guoye-zhang guoye-zhang guoye-zhang approved these changes

Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

🆕 semver/minor Adds new public API.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@gjcairo @guoye-zhang @0xTim @aryan-25